Update aggs reference documentation for 'keyed' options#23758
Update aggs reference documentation for 'keyed' options#23758javanna merged 5 commits intoelastic:masterfrom
Conversation
Add 'keyed' parameter documentation for following: - Date Histogram Aggregation - Date Range Aggregation - Geo Distance Aggregation - Histogram Aggregation - IP range aggregation - Percentiles Aggregation - Percentile Ranks Aggregation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
Many thanks for this. |
|
@markharwood I saw those tags while browsing the docs but wasn't sure about their purpose. Additionally, I copied the documentation from Range Aggregation(which has docs for |
|
It's an evolution we're on - runnable/testable docs is the goal and it will take us some time to get 100% coverage of old docs but we are aiming for all new docs to have this as part of the examples.
The Thanks again |
|
@markharwood If I have the failure logs like so: How do I run the gradle check only for the required file? I could not find the line with Running |
|
Here's an example from the last aggregation I worked on: and then run You can either repeat this for your example JSON or copy the |
|
Histogram Aggregation already has a section 'Response Format' which addresses the same topic. Should remove the added |
This worked. It turns out, the reproduce with was printed but since there was so much log output, it got gobbled up. |
Add testable examples using indexes populated in build.gradle - Date Histogram Aggregation - Date Range Aggregation - Percentiles Aggregation - Percentile Ranks Aggregation Remove redundant section `Keyed Response`
|
I have updated the docs with testable examples as requested with a couple of exceptions. When I run I have not made any change to that file so merging this should not be a problem. EDIT: Post merging master, I was able to see that Geodistance Aggregation had been updated and now had sample data. I used the same for updating the examples so that they are testable as well. |
|
hi @sudo-suhas this was open a while ago, would you mind rebasing your PR against latest master please? |
|
There was a conflict in one of the files and I merged the master branch using the GUI while resolving it in github itself. Do I still need a rebase? |
|
no sorry @sudo-suhas I didn't notice that. I will run tests then. |
|
jenkins please test this |
javanna
left a comment
There was a problem hiding this comment.
hi @sudo-suhas this looks great! I left a minor comment, if you can address that I can then merge this in.
|
|
||
| Response: | ||
|
|
||
| Response: |
There was a problem hiding this comment.
can you remove this "Response:" repetition?
There was a problem hiding this comment.
My bad. Pushed commit with the fix
Add 'keyed' parameter documentation for following: - Date Histogram Aggregation - Date Range Aggregation - Geo Distance Aggregation - Histogram Aggregation - IP range aggregation - Percentiles Aggregation - Percentile Ranks Aggregation
Add 'keyed' parameter documentation for following: - Date Histogram Aggregation - Date Range Aggregation - Geo Distance Aggregation - Histogram Aggregation - IP range aggregation - Percentiles Aggregation - Percentile Ranks Aggregation
|
Thanks again @sudo-suhas I merged this to master and cherry-picked to 5.x and 5.4. |
|
Thank you for suggesting I make a pull request 😉 |
The
keyedparameter is supported by the following aggregations:However, in the reference guide, the
keyedparameter is documented only for Range Aggregation.This PR adds the documentations for the rest.
Closes #23731.